Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documenting the common parameter CAM_MODE. #164

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

dogmaphobic
Copy link
Contributor

No description provided.

@@ -97,6 +97,7 @@ CAM_EV | Exposure Compensation (usually only used for automatic exposure modes.)
CAM_EXPMODE | Exposure Mode (Manual, Auto, Program Auto, Aperture Priority, etc.)
CAM_ISO | ISO
CAM_METERING | Metering Mode
CAM_MODE | Camera Mode. Invariably, when switching modes, some of the camera parameters that are available for one mode is not available for the other. Video mode may offer video recording resolutions while photo mode may offer image capture size, format, compression, etc. If you simply use the MAVLink command [MAV_CMD_SET_CAMERA_MODE](../messages/common.md#MAV_CMD_SET_CAMERA_MODE), you won't have a chance to let the GCS know that. If your camera does have different parameters and/or options for [CAMERA_MODE_IMAGE](../messages/common.md#CAMERA_MODE) or [CAMERA_MODE_VIDEO](../messages/common.md#CAMERA_MODE), you may want to use this parameter (along with the proper exclusion rules) instead. The CGS will look to see if this parameter is exposed. If it is, it will use it instead of [MAV_CMD_SET_CAMERA_MODE](../messages/common.md#MAV_CMD_SET_CAMERA_MODE). If the parameter is not exposed, [MAV_CMD_SET_CAMERA_MODE](../messages/common.md#MAV_CMD_SET_CAMERA_MODE) will be used instead. See an example of a definition of `CAM_MODE` below.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo:: one mode are not available for the other.

Copy link
Collaborator

@hamishwillee hamishwillee Jan 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Camera Mode Setting Options. This parameter allows you to specify the settings (and their options) that can be set for image capture mode and for video capture mode (CAMERA_MODE). The parameter might be used to specify image capture sizes, image/video formats, compression levels, ISO values, shutter speeds, etc. that are relevant to each of the modes. If CAM_MODE is not defined then the camera will not have mode-configurable settings.
There is an example CAM_MODE definition below.

Note, if you need an anchor for the cam_mode_example you can get an anchor from a heading or make one using a span - e.g. <span id="cam_mode_example"></span>.

Copy link
Collaborator

@hamishwillee hamishwillee Jan 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more thoughts

  • There is just one part I may have missed, and that is that if you have (say) CAM_SHUTTERSPD but not CAM_MODE, my understanding is that CAM_SHUTTERSPD would be used in all modes. Correct?
  • Is the name of this parameter fixed? CAM_MODE is not very descriptive. Perhaps CAM_MODE_SETTINGS

If I'm correct about the first point above, how about:

Camera Mode Setting Options. This parameter allows you to specify the settings (and their options) that can be set for image capture mode and for video capture mode (CAMERA_MODE). The parameter might be used to specify image capture sizes, image/video formats, compression levels, ISO values, shutter speeds, etc. that are relevant to each of the modes. If CAM_MODE is not defined then the camera will not have mode-configurable settings - settings like CAM_SHUTTERSPD will apply to all modes (if defined in both places, the CAM_MODE setting is used).
There is an example CAM_MODE definition below.

@dogmaphobic
Copy link
Contributor Author

Let me trying explaining it using an actual (simplistic) example. Let’s assume you have a camera that has both Video and Photo mode. The camera also allows setting ISO, Video Recording Resolution and Photo File Format.

Video Recording Resolution is only relevant when in Video Mode
Photo File Format is only relevant when in Photo Mode

In addition, the camera range of ISO options is limited when in Video Mode.

The camera definition would expose (very condensed for simplicity’s sake):

Video Recording Resolution

<parameter name="CAM_VIDRES" type="uint32" default="0">
    <description>Video Resolution</description>
    <options>
        <option name="4096 x 2160 60fps (UHD)" value="0" />
        <option name="4096 x 2160 50fps (UHD)" value="1" />
        <option name="4096 x 2160 48fps (UHD)" value="2" />
        <option name="4096 x 2160 30fps (UHD)" value="3" />
        <option name="4096 x 2160 25fps (UHD)" value="4" />
        <option name="4096 x 2160 24fps (UHD)" value="5" />
        <option name="3840 x 2160 60fps (UHD)" value="6" />
        <option name="3840 x 2160 50fps (UHD)" value="7" />
        <option name="3840 x 2160 48fps (UHD)" value="8" />
        <option name="3840 x 2160 30fps (UHD)" value="9" />
        <option name="3840 x 2160 25fps (UHD)" value="10" />
        <option name="3840 x 2160 24fps (UHD)" value="11" />
    </options>
</parameter>

Photo File Format

<parameter name="CAM_PHOTOFMT" type="uint32" default="0">
    <description>Image Format</description>
    <options>
        <option name="Jpeg" value="0" />
        <option name="Raw" value="2" />
    </options>
</parameter>

ISO

<parameter name="CAM_ISO" type="uint32" default="100">
    <description>ISO</description>
    <options>
        <option name="100" value="100" />
        <option name="150" value="150" />
        <option name="200" value="200" />
        <option name="300" value="300" />
        <option name="400" value="400" />
        <option name="600" value="600" />
        <option name="800" value="800" />
        <option name="1600" value="1600" />
        <option name="3200" value="3200" />
        <option name="6400" value="6400" />
    </options>
</parameter>

You want to tell the GCS to exclude Video Recording Resolution when in Photo Mode, exclude Photo File Format when in Video Mode, and you also want to tell the GCS that when in Video Mode, the range of options for ISO is different:

<parameter name="CAM_MODE" type="uint32" default="1" control="0">
    <description>Camera Mode</description>
    <options>
        <option name="Photo" value="0">
            <!-- This tells us when Camera Mode is set to Photo mode, the following parameters should be ignored (hidden from UI or disabled)-->
            <exclusions>
                <exclude>CAM_VIDRES</exclude>
            </exclusions>
        </option>
        <option name="Video" value="1">
            <!-- Converselly, when Camera Mode is set to Photo mode, the following parameters should be ignored (hidden from UI or disabled)-->
            <exclusions>
                <exclude>CAM_PHOTOFMT</exclude>
            </exclusions>
            <parameterranges>
                <!-- Here we tell the GCS that when in Video Mode, the range of ISO options is different. We give a new range.-->
                <parameterrange parameter="CAM_ISO">
                    <roption name="100" value="100" />
                    <roption name="150" value="150" />
                    <roption name="200" value="200" />
                    <roption name="300" value="300" />
                    <roption name="400" value="400" />
                    <roption name="600" value="600" />
                    <roption name="800" value="800" />
                    <roption name="1600" value="1600" />
                    <roption name="3200" value="3200" />
                </parameterrange>
            </parameterranges>
        </option>
    </options>
</parameter>

The above will cause the GCS to hide Video Recording Resolution when the mode is set to Photo Mode and conversely, hide Photo File Format when in Video Mode. In addition, the GCS will show the full set of ISO options when in Photo Mode but only a subset when in Video Mode.

@hamishwillee
Copy link
Collaborator

hamishwillee commented Jan 15, 2019

@dogmaphobic Thanks for the detailed example: #164 (comment). The fix is to add your example to the docs under own heading. The text for this parameter would then be much simpler:

Camera Mode Setting Configuration. This parameter allows you to specify the camera settings and options that apply for the different camera modes (i.e. in image capture and in video capture mode). There is an example CAM_MODE definition below.

This is better because it states the bare minimum. I have explicitly used the word "apply" because I don't want to go into the detail about excluding or include particular "things" at this point.

The example section would also make the statement about what happens if CAM_MODE is not set - relevant camera settings apply to both modes.

Does that work for you?

@dogmaphobic
Copy link
Contributor Author

Camera Mode Setting Configuration. This parameter allows you to specify the camera settings and options that apply for the different camera modes (i.e. in image capture and in video capture mode).

Let's back up a bit. That's not quite accurate. We got way too deep into the reasons you might want to have this parameter defined but the explanation of what it does was lost in the process.

Most cameras have two modes of operation: Photo and Video modes. There is a specific set of MAVLInk commands to get/set the camera mode. However, if you need to have a different set of parameters and options depending on the mode, you can only do that by using a CAM_PARAMETER instead of using the MAV_CMD_SET_CAMERA_MODE command.

That's basically what this is trying to explain.

@hamishwillee
Copy link
Collaborator

Most cameras have two modes of operation: Photo and Video modes. There is a specific set of MAVLInk commands to get/set the camera mode. However, if you need to have a different set of parameters and options depending on the mode, you can only do that by using a CAM_PARAMETER instead of using the MAV_CMD_SET_CAMERA_MODE command.

@dogmaphobic Nope, clear as mud. The problem is the last line and the use of the word "instead".

What the above says is that if you want to have a different set of parameters on modes you have to use CAM_MODE and NOT use MAV_CMD_SET_CAMERA_MODE.

But that makes no sense because:

  • you still have to use MAV_CMD_SET_CAMERA_MODE to set the camera into a particular mode (at which point you benefit from whatever params are set).
  • MAV_CMD_SET_CAMERA_MODE only sets the mode - it doesn't set parameters, and the implication of the statement is that you can use MAV_CMD_SET_CAMERA_MODE to set parameters.

So for "instead" above to be true there must be some other way to change the mode than calling MAV_CMD_SET_CAMERA_MODE.

Does this explain my confusion?

@dogmaphobic
Copy link
Contributor Author

  • you still have to use MAV_CMD_SET_CAMERA_MODE to set the camera into a particular mode (at which point you benefit from whatever params are set).

Bingo. No, you don't. To set the camera mode you either use one or the other. For simple cameras with no parameters (no camera definition file or very simple parameters that don't change between modes), you use MAV_CMD_SET_CAMERA_MODE. Otherwise, you use PARAM_EXT_SET for the CAM_MODE parameter. They are mutually exclusive.

When the user hits the mode change button (toggling between Photo and Video), the code (in QGC) first looks to see if the camera exposes a CAM_MODE parameter. If it does, it uses it to set the mode. If the camera does not expose CAM_MODE, it will use MAV_CMD_SET_CAMERA_MODE instead.

https://github.com/mavlink/qgroundcontrol/blob/cameraAndVideo/src/Camera/QGCCameraControl.cc#L443-L468

The reason why your camera would use one instead of the other is what this whole conversation above went all about.

@hamishwillee
Copy link
Collaborator

Thanks. YOu must think I'm dumb as a stump. I'll try reword after lunch.

@dogmaphobic
Copy link
Contributor Author

YOu must think I'm dumb as a stump. I'll try reword after lunch.

Heck no 🙃 This is awfully confusing for anybody unless you live and breathe cameras' user interfaces. I've been doing exactly this for far longer that I would like to admit so for me, it's all second nature. Until I have to explain it. Then it gets where we are 🤪

@hamishwillee
Copy link
Collaborator

OK, let's try this again :-)

Camera Mode. This parameter may be used to specify the different camera configuration settings (ISO, video/image resolution, frame rates, encoding format, etc.), and the valid options within those settings, that will apply for each of the camera modes (i.e. for image capture and video capture). There is an example CAM_MODE definition with additional detail below.

If defined, the CAM_MODE parameter must be used to set the current camera mode (via PARAM_EXT_SET). If it is not defined then MAV_CMD_SET_CAMERA_MODE may be used to set the mode instead, and any configuration settings/options will be the same for all modes.

Better?

I assume CAM_MODE mode options have match the CAMERA_MODE enum? Ie could you specify "FooMode" and would everything fall over?

@dogmaphobic
Copy link
Contributor Author

Better?

In ways, but it's explaining the reason and not the function. CAM_MODE is used to change modes. That is its function. The reason to use it instead of the MAV_CMD_SET_CAMERA_MODE command is what you described.

I assume CAM_MODE mode options have match the CAMERA_MODE enum? Ie could you specify "FooMode" and would everything fall over?

Correct. QGC only knows CAMERA_MODE_IMAGE (0) and CAMERA_MODE_VIDEO (1). Those are the only two options it will use and those are the values that should be defined for each option. Any other option will be ignored.

@hamishwillee
Copy link
Collaborator

No worries, just invert:

Camera Mode. This parameter is used to change the camera mode (via PARAM_EXT_SET) for cameras that support different settings/options (ISO, video/image resolution, frame rates, encoding format, etc.) for image and video capture camera modes.

If it is not defined then MAV_CMD_SET_CAMERA_MODE can be used to set the mode instead (this command is ignored if CAM_MODE is defined), and any configuration settings/options will be common to all modes.

There is an example CAM_MODE definition with additional detail below.

We'd then include your example in detail further down as the link, and remember to state the things that are "required" like the fact that it only knows about the image and video modes.

@hamishwillee
Copy link
Collaborator

hamishwillee commented Jan 18, 2019

@dogmaphobic I'm off for a week. I think my text here is OK: #164 (comment) and you could add your example more or less "as is".

Upshot, if you need to merge before I get back then I can always update if needed as a post process.

@dogmaphobic
Copy link
Contributor Author

Sorry, I've been buried with other stuff. Yup, inverting it is perfect and I'm fine with what we have. We can always revisit it later.

@hamishwillee
Copy link
Collaborator

Hi @dogmaphobic ,

OK, I added the cam-mode text and linked down to the "Range Limit" section for the example - please check: 93657af

Do we need to do anything else before merging this one? (and do you want me to do so)?

@hamishwillee
Copy link
Collaborator

@dogmaphobic Ping - this ready to progress?

@dogmaphobic
Copy link
Contributor Author

Enough?

Yup! :)

@olliw42
Copy link
Contributor

olliw42 commented May 20, 2019

thx for the explanations in the above, they made the use of that CAM_MODE and when it is needed very clear.

I'm however not totally happy with requiring that MAV_CMD_SET_CAMERA_MODE and PARAM_EXT_SET are mutually exclusive. I in fact can't see any reason for this restriction, or advantage of it, or reason why they shouldn't be allowed to be used both.

From the usage perspective I think it's obvious that the logic would be easier if both could be used: One always can use MAV_CMD_SET_CAMERA_MODE, while PARAM_EXT_SET can be used only if a CAM_MODE parameter is existing, which is all as normal. Simpler usage logic implies easier documentation.

From the GCS implementation perspective I can't see any disadvantage or reason either. Any GCS can decide to do as it wants, i.e. QGC for example doesn't have to do anything differently. However, I see advantages, e.g., scripts which just want to do mode changes and not also all the fancy xml stuff would become much simpler, and more generic, since they now just could call MAV_CMD_SET_CAMERA_MODE and wouldn't have to bother with first testing for the existence of CAM_MODE and then to switch code path.

Also from the cameras implementation I can't see any disadvantage. Any camera supporting mode change will, in one way or the other, have some internal mode field anyhow. The only difference would be if it is exposed as parameter or not. The one tiny argument I can see which remotely could be regarded as a disadvantage is that a CAM_MODE-camera would have to add code for supporting MAV_CMD_SET_CAMERA_MODE , which however are literally only 4 lines of code, and thus should be really marginal (piece of cake).

I think this are compelling reasons, and thus would remove the mutually exclusive requirement. Instead I would specify that MAV_CMD_SET_CAMERA_MODE should always be supported, and cameras which want to make use of the techniques you described and which require a CAM_MODE parameter should in addition provide the mode variable as such parameter.

cheers, Olli

@dogmaphobic
Copy link
Contributor Author

I'm however not totally happy with requiring that MAV_CMD_SET_CAMERA_MODE and PARAM_EXT_SET are mutually exclusive. I in fact can't see any reason for this restriction, or advantage of it, or reason why they shouldn't be allowed to be used both.

QGC won't send the mode change command twice (using the MAVLink command AND the CAM_MODE parameter.) Is that you are asking for? If you don't want the parameter, don't expose it. If you expose it, you must have a reason to do so and that reason precludes the use of the MAVLink command from being used. Won't it?

@olliw42
Copy link
Contributor

olliw42 commented May 20, 2019

QGC won't send the mode change command twice (using the MAVLink command AND the CAM_MODE parameter.) Is that you are asking for?

no, this wasn't it, I'm of course not expecting any GCS to send anything else that just one message for each mode change

but what I'm saying is that, for a camera having a CAM_MODE parameter, it should be "either a MAV_CMD_SET_CAMERA_MODE or a PARAM_EXT_SET", and not be "only a PARAM_EXT_SET but not a MAV_CMD_SET_CAMERA_MODE" like your statements imply.

If you expose it, you must have a reason to do so

yo

and that reason precludes the use of the MAVLink command from being used. Won't it?

why? I fail to see any reason for that restriction to be required here (except of this tiny counter argument which I rated as marginal)

@dogmaphobic
Copy link
Contributor Author

And how would QGC know to use the command instead of the exposed CAM_MODE?

@olliw42
Copy link
Contributor

olliw42 commented May 20, 2019

it doesn't need to know, QGC can happily continue to do things exactly as it is doing it now! No change at all required. I'm talking about an OR!

@olliw42
Copy link
Contributor

olliw42 commented May 20, 2019

let me make a truth table for your exclusive statement:

  • camera WITHOUT xml/CAM_MODE: use MAV_CMD_SET_CAMERA_MODE but not PARAM_EXT_SET (as this doesn't make sense)

  • camera WITH xml/CAM_MODE: do not use MAV_CMD_SET_CAMERA_MODE but use PARAM_EXT_SET

for comparison this is the truth table I suggest:

  • camera WITHOUT xml/CAM_MODE: use MAV_CMD_SET_CAMERA_MODE but not PARAM_EXT_SET (as this doesn't make sense)

  • camera WITH xml/CAM_MODE: use MAV_CMD_SET_CAMERA_MODE or use PARAM_EXT_SET

as simple as that

@hamishwillee
Copy link
Collaborator

@dogmaphobic I'm not tracking this, is there an action for me :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants